Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Search Sessions] Don’t try deleting errored searches #105434

Merged
merged 3 commits into from
Jul 15, 2021

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Jul 13, 2021

Summary

Partially addresses #104309

There are cases when searches in es and search session object get out of sync. For example, when leaving a dashboard while it is loading without saving a session.

In this case, when we clean up a session we will also try to delete an errored-out search. I think we can avoid this request altogether.

longer-term we should find a way to avoid getting in an out-of-sync state. This could possibly be addressed with some variation of #99658, which has some unsolved problems atm. Filed an issue: #105441

Checklist

@Dosant Dosant added bug Fixes for quality problems that affect the customer experience v7.14.0 impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:AppServices Feature:Search Querying infrastructure in Kibana release_note:fix labels Jul 13, 2021
@Dosant Dosant marked this pull request as ready for review July 13, 2021 13:32
@Dosant Dosant requested a review from a team as a code owner July 13, 2021 13:32
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@Dosant Dosant requested review from lizozom and lukasolson July 13, 2021 13:32
Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the simple approach on this one.
Tested and didn't see any errors in the console for abandoned sessions (even if the search was canceled).

Also strongly agree with the longer term approach

@Dosant
Copy link
Contributor Author

Dosant commented Jul 15, 2021

@elasticmachine merge upstream

@Dosant Dosant added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 15, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.14

This backport PR will be merged automatically after passing CI.

@Dosant Dosant added v7.15.0 v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Jul 15, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 15, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 15, 2021
…-png-pdf-report-type

* 'master' of github.com:elastic/kibana: (75 commits)
  [Search Sessions] Don’t try to delete errored searches (elastic#105434)
  [Search Sessions] fix saved object can be created even if courier:batchSearches is enabled (elastic#105407)
  [Remote Clusters] Fixed remote clusters details flyout for long strings (elastic#105592)
  [ML] Functional tests - re-activate a11y tests (elastic#105198)
  [APM] Typed client-side routing (elastic#104274)
  [Canvas] Expression error (elastic#103048)
  [ML] Fixing job wizard with missing description (elastic#105574)
  [Security Solution][Alerts] - Add alerts subfeature UI (elastic#105505)
  Upgrade EUI to v35.0.0 (elastic#105127)
  [Reporting] Clean up types for internal APIs needed for UI (elastic#105508)
  skip flaky suite (elastic#105087)
  [Workplace Search] Fix Chrome issues with GitHub sources (elastic#105680)
  [Fleet] Add containerized fleet server instructions to Fleet README (elastic#105669)
  [ML] Add api integration test for analytics map endpoint  (elastic#105531)
  Fixes cypress flake across two tests (elastic#105645)
  [Logs&Metrics UI] add owner properties to plugin manifest (elastic#105580)
  chore(NA): introduce preset for jest-integration tests on @kbn/test (elastic#105144)
  [Enterprise Search] Added Thumbnails to Search UI (elastic#104199)
  Translate App Search credentials list (elastic#105619)
  [APM] APM agent config created prior to Fleet migration is not injected into integration policy (elastic#105504)
  ...

# Conflicts:
#	x-pack/plugins/reporting/common/types.ts
#	x-pack/plugins/reporting/public/management/report_listing.test.tsx
kibanamachine added a commit that referenced this pull request Jul 15, 2021
…5734)

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
kibanamachine added a commit that referenced this pull request Jul 15, 2021
…5737)

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience Feature:Search Querying infrastructure in Kibana impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. release_note:fix v7.14.0 v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants